Skip to content

Revert pr 2025#2041

Merged
gmechali merged 3 commits into
datacommonsorg:masterfrom
gmechali:revert-pr-2025
May 28, 2026
Merged

Revert pr 2025#2041
gmechali merged 3 commits into
datacommonsorg:masterfrom
gmechali:revert-pr-2025

Conversation

@gmechali
Copy link
Copy Markdown
Contributor

@gmechali gmechali commented May 27, 2026

This is a reversion of #2025, however, I just kept the env varaible propagated through.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies SQL query generation in aggregation_utils.py by removing single-quote escaping on import_names and hardcoding the 'dc/base/' prefix and 'dc/base/GeneratedGraphs' provenance. The reviewer points out that these changes introduce critical SQL injection vulnerabilities because user-controlled input is no longer escaped. Additionally, hardcoding the base prefix breaks functionality for non-base Data Commons instances where self.is_base_dc is False.

Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
Comment thread import-automation/workflow/ingestion-helper/aggregation_utils.py
@gmechali
Copy link
Copy Markdown
Contributor Author

Thanks Christie!

@SandeepTuniki Can you (and Vishal) pls take over reverting this PR + making the dataflow work without adding dc/base ? You can now test the e2e DCP setup to ensure this will work :)

@gmechali
Copy link
Copy Markdown
Contributor Author

I resolved all the comments even though they are real injection vulnerabilities. Sandeep, when you take this over, can you ensure to re-resolve the vulns?

@gmechali gmechali merged commit 5ed69c0 into datacommonsorg:master May 28, 2026
13 checks passed
@gmechali gmechali deleted the revert-pr-2025 branch May 28, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants